Skip to content

Make propose and commit agree on time-varying authority - #107

Merged
b-macker merged 1 commit into
masterfrom
claude/naab-inadmissible-action-prevention-4cmn1m
Jul 30, 2026
Merged

Make propose and commit agree on time-varying authority#107
b-macker merged 1 commit into
masterfrom
claude/naab-inadmissible-action-prevention-4cmn1m

Conversation

@b-macker

Copy link
Copy Markdown
Owner

Summary

#105 established the invariant: every check whose truth can change with the passage of time belongs on the commit half, because authority is time-varying and a proposal is not. Two holes in that invariant remained, both on the propose/commit path.

A1 — proposals now carry the config generation they were evaluated under

agent.commit() deliberately does not call reloadIfChanged() — traced and rejected in #105, because an accepted reload runs onAgentConfigChanged(), which re-baselines mandate_keywords and would score an already-generated candidate against a mandate it never received, besides dangling config through the atomic rules_ptr_ swap. That decision was right, but its consequence was left open: a candidate produced under one configuration could still be committed under another.

Proposals are now stamped with reload_count_ at propose time and commit refuses when it has moved — declining to commit under conditions the candidate never saw, rather than re-scoring it under them.

reload_count_ and not governance_epoch_: the epoch also increments on pulse verdict transitions (governance_engine.cpp:6906), and the pulse sawtooths, so proposals would be voided during normal degraded operation.

This was not additive. reload_count_ was a plain int (governance.h:3478) while governance_epoch_ beside it is std::atomic<int>. It is written under reload_mutex_ but getReloadCount() read it unsynchronised — reading it from agent worker threads during batch/fan_out would have been a data race. It is now atomic. The accessor had no callers, so the change is contained to four sites.

A2 — propose consults the lease even with step-up disabled

agentPropose's lease test sat inside if (cb.step_up_enabled), and step_up_enabled defaults to false (governance.h:1600). So by default propose did not consult the lease at all, while agent.send() hard-throws (agent_impl.cpp:1461) and agent.commit() checks it unconditionally (#105). Propose was the only entry point where expired authority passed silently.

The lease test is now unconditional. The level test stays inside the guard — required_level derives from step_up_at_level, so it is intrinsically about step-up and means nothing when step-up is off. Refusal wording matches the two existing shapes so callers keep matching on them.

Regression surface is empty, verified. Only four configs in the repo enable propose:

config step_up lease
test_split_commit.sh absent → false none
test_absorption_degenerate.sh absent → false none
test_propose_commit.sh true configured
living-script_extended/src/govern.json true configured

Nothing combines propose + a lease + step-up off. The two with step-up off have no lease, so lease_configured is false and the hoisted check is a no-op for them. The change reaches only the configuration nobody was using — which is the hole.

Two mechanics that cost real time to find

Both are recorded in the test, since anyone writing a similar case will hit them:

  • reloadIfChanged() compares mtime at second granularity (governance_config.cpp:3753). A rewrite landing in the same second as the initial load is invisible — the first attempt at H-01 silently did nothing.
  • Subprocess env is scrubbed, so a re-sign must be passed --signing-key explicitly; NAAB_SIGNING_KEY does not survive into the child. This is the same workaround living-script.naab already uses.

Test Plan

  • test_propose_commit.sh23/23, Groups A–G unchanged, H and I new
  • Vacuity-checked, the signature that matters being which assertion fails:
    • stamp comparison disabled → H-01 fails, H-02 still passes (the no-change control survives)
    • old step_up_enabled nesting restored → I-01 fails, I-02 and I-03 still pass (the no-op and anti-regression controls survive)
  • tests/security/test_error_msg_leaks.sh — 874/0 (new refusal text)
  • bash run-all-tests.sh441 tests, 0 unexpected failures
  • asan/ubsan — not runnable locally without a separate sanitizer build; CI covers both, and they are the jobs that would catch the reload_count_ atomic change being wrong. Worth watching on this PR specifically.

Honest scoping

The reload stamp will rarely fire in living-script_extended — nothing between propose, select_admissible and commit triggers a reload there, and the one path that does send (the #106 re-auth retry) already invalidates the proposal. This is an invariant, not a detector. It earns its place in multi-agent scripts where a sibling agent's send, a polyglot block (polyglot.cpp:158) or the VM (vm.cpp:2815) reloads mid-gap.

PR B (example taint/integrity coverage) is deliberately separate and does not depend on this.


Generated by Claude Code

#105 established that every check whose truth can change with the passage of
time belongs on the commit half, because authority is time-varying and a
proposal is not. Two holes in that invariant remained, both on this path.

Config changes were not re-checked across the propose→commit gap. commit()
deliberately does not call reloadIfChanged() — an accepted reload runs
onAgentConfigChanged(), which re-baselines mandate_keywords and would score an
already-generated candidate against a mandate it never received, besides
dangling `config` through the atomic rules_ptr_ swap. That decision was right,
but its consequence was left open: a candidate produced under one configuration
could still be committed under another. Proposals now carry the accepted-config
generation they were evaluated under, and commit refuses when it has moved —
declining to commit under conditions the candidate never saw, rather than
re-scoring it under them.

reload_count_ and not governance_epoch_: the epoch also moves on pulse verdict
transitions, and the pulse sawtooths, so proposals would be voided during
normal degraded operation.

That stamp is not additive. reload_count_ was a plain int while
governance_epoch_ beside it is atomic; it is written under reload_mutex_ but
getReloadCount() reads it unsynchronised, so reading it from agent worker
threads during batch/fan_out would have been a data race. It is now
std::atomic<int>. The accessor had no callers, so the change is contained to
four sites.

Second hole: agentPropose's lease test sat inside `if (cb.step_up_enabled)`,
and step_up_enabled defaults to false — so by default propose did not consult
the lease at all, while send() hard-throws on an expired lease and commit()
checks it unconditionally. Propose was the only entry point where expired
authority passed silently. The lease test is now unconditional; the level test
stays inside the guard because required_level derives from step_up_at_level and
means nothing when step-up is off. Refusal wording matches the two existing
shapes so callers keep matching on them.

No config in the repo combines propose with a lease and step-up off — the two
propose configs with step-up off have no lease at all, so the hoisted check is
a no-op for them. The change reaches only the configuration nobody was using,
which is the hole.

Groups H and I cover both. Two mechanics cost real time to find and are
recorded in the test: reloadIfChanged() compares mtime at second granularity,
so a rewrite in the same second is invisible; and subprocess env is scrubbed,
so the re-sign must be passed --signing-key explicitly.

Vacuity-checked: disabling the stamp fails H-01 while H-02 still passes,
restoring the old nesting fails I-01 while I-02 and I-03 still pass.

Full suite: 441 tests, 0 unexpected failures.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ELUfjXZvx8kzXo1UJjrAhC
@github-actions

Copy link
Copy Markdown

NAAb Governance Report

Metric Count
Files checked 16
Passed 16
Failed 0

All governance checks passed!

Generated by NAAb Governance Engine v4.0

@b-macker
b-macker marked this pull request as ready for review July 30, 2026 23:43
@b-macker
b-macker merged commit f357275 into master Jul 30, 2026
23 checks passed
@b-macker
b-macker deleted the claude/naab-inadmissible-action-prevention-4cmn1m branch July 30, 2026 23:44
b-macker added a commit that referenced this pull request Aug 1, 2026
checkAdmission() denies at CRITICAL with a HARD block — "all autonomous actions
suspended until pressure subsides" — and agent.propose() is gated on it through
that call. agent.commit() re-checked the handle, max_turns, the lease (#105) and
the config generation (#107), but never the level. Commit is where the proposed
transition becomes real: history is appended and the turn advances. So a
candidate generated at NORMAL could still land after the engine reached CRITICAL.

The proposing handle need do nothing for this to happen. governance_level_ is a
single engine-global atomic written from CDD processing, driven by whichever
handle last took a turn, while s_pending_proposals is keyed per handle — so only
that handle's own activity invalidates its proposal. A sibling agent's
misbehaviour therefore escalates across the deliberation gap while the proposal
stays valid, which is exactly the shape batch, fan_out and pipelines run in.

The reasoning that justifies commit's other omissions does not extend here, and
that is the trap worth naming: hard_stopped and recordAutonomousAction are left
out because they guard SPEND and commit makes no API call. A CRITICAL suspension
is not a spend guard. Hence checkCriticalSuspension(), the CRITICAL branch split
out of checkAdmission() into its own public method, rather than calling
checkAdmission() wholesale — the latter also projects one more autonomous action
against max_autonomous_actions and would refuse a commit that costs nothing.
The refusal is a GovernanceHardError, matching admission's enforcement at
propose. Same invariant #105 and #107 established: a check whose truth changes
with the passage of time belongs on the commit half.

Group J covers it, with J-02 as the control — without escalation the same commit
must succeed, or J-01 shows only that commits fail. Two staging traps are pinned
in the test and the docs because both produced a green that meant nothing:

The escalation is two thresholds in series. consecutive_high_pressure_turns only
increments while composite >= reality_checkpoint.pressure_threshold, and the
CRITICAL target needs composite >= critical_threshold AND consecutive >=
critical_sustained. Setting only the circuit-breaker half pins the counter at
zero and the level never moves.

The first working version passed with the fix reverted. The sibling sent twice,
and the second send's own checkAdmission() hard-blocked once the first had
escalated — the process died before the commit, stderr still said CRITICAL, and
the assertion matched a blocked sibling send rather than the gate under test.
One send plus a PRE_COMMIT marker distinguishes "blocked earlier" from "blocked
at commit".

Live status: stub-only. No live run has reached CRITICAL with a proposal
outstanding, so the firing path has not executed against a real API.


Claude-Session: https://claude.ai/code/session_01ELUfjXZvx8kzXo1UJjrAhC

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants